I copied this from ReplayTest but why are we even running these on emulators in gh actions?

💯

l: We could beef this up to protect against misuse if we think it'd help:

 * <p>**Bitmap lifecycle:** The bitmap is owned by the replay system. It should be deemed 
 * read-only and valid solely for the duration of the callback. Do not write to it or recycle it. Do
 * not synchronize on it, store a reference to it, or access it after this method returns – copy
 * the pixel data (e.g., compress to a file) within this method if you need it later.
 *
 * <p>The callback runs on a background thread (the replay executor).

...or whatever makes sense.

i've updated it to copy the bitmap. let me know if you think the new docs make sense along with that change to copy it

I need to dig deeper in to why we need to add this to all the modules that add libs.jetbrains.annotations another day.

I would not have thought of calling copy.recycle() if we get an exception here. AI is smart.

with the bitmap copy, we now have to make sure that we call bitmap.recycle() as should consumers of our API.

Unguarded NPE from bitmap.config!! skips frame storage

Medium Severity

The bitmap.config!! non-null assertion on line 316 is outside the try-catch block. Android's Bitmap.getConfig() can return null (e.g., for certain decoded image formats). If it does, a KotlinNullPointerException propagates out of the lambda, preventing addFrame on line 326 from ever executing. The ReplayExecutorService catches the exception so it won't crash, but the replay frame is silently lost. Setting a snapshotObserver can degrade replay quality even when the observer itself is well-behaved.

 

^{Reviewed by Cursor Bugbot for commit 4a829fe. Configure here.}

just FYI, this is a pretty contrived test but it just makes sure we can capture a replay using the new snapshot observer api

this awkward API is the consequence of moving the API to the sentry-andorid-replay api

we copy the bitmap now

no point in calling the API if we have a null bitmap

❗- Ah, I thought we were going the SentryReplayOptions route to avoid allowing folks to mutate configs after Sentry.init() returns (+ for enhanced visibility and better ergonomics)?

Oh, then I misunderstood, this is the same API as in your branch which is what I thought we agreed to: a97de37#diff-cb8ccc45cef66aca485d205c9e99c17c114b6319ab9b7d52173b52ee72b047a8R126

We just need to make it public here instead of private in your branch because it needs to be usable from other packages i.e. other apps.

@Experimental

We should have our try block below enclose bitmap.copy(), esp given the !! 👍

I think the only types of errors that can happen when copying the bitmap are memory errors and we shouldn't try to catch those.
As for the !! I don't see how bitmap.config can be null if it is a valid bitmap. Do you know how it could be null?

❗- It's not in the diff, so apologies for missing it the first time round, but I see there's an onScreenshotRecorded(File, Long) method as well as the Bitmap version above. I imagine we need to call the observer in both....

Worth emphasizing that folks should keep processing quick or hand off to another thread. (Otherwise they'll slow down the SDK's replay pipeline.)

Newbie question, but are we able to observe the performance hit of copying on our end?

Esp if we want to expand our use-cases beyond testing or debugging at some point, it'd be good to know whether we need to optimize.

Memory-wise, each bitmap is roughly 900kb and we produce one frame per second.

Worth noting that this is for use in snapshot tests and for debugging purposes and isn't currently optimized for production use.